Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(NODE-3333): support 'let' option for CRUD commands #2829

Merged
merged 3 commits into from
Jun 8, 2021

Conversation

addaleax
Copy link
Contributor

@addaleax addaleax commented Jun 7, 2021

Description

What changed?

Same as #2828.

Are there any files to ignore?

The spec tests here are fake in the sense that no upstream spec tests
for this exist at this point. I’ve put them next to the other tests, more or less
just assuming that somebody would notice their existence when updating
the crud spec suite to upstream and make sure they aren’t deleted.
(They are just a temporary thing here anyway.)

(The spec tests here are fake in the sense that no upstream spec tests
for this exist at this point.)
@nbbeeken
Copy link
Contributor

nbbeeken commented Jun 8, 2021

Looks like the aggregate-let tests are failing, there's an update in the spec that I think fixes it. Or you could skip it for now in:
test/functional/crud_spec.test.js:447
change to:

await runUnifiedTest(this, crudSpecTest, test, ['Aggregate with let option']);

@addaleax
Copy link
Contributor Author

addaleax commented Jun 8, 2021

Err, sure, I’ll update the tests – it’s unrelated to the rest of the changes here, though – let me know if I should push them to a separate PR?

Copy link
Contributor

@nbbeeken nbbeeken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can close the related ticket (NODE-3334) and link this PR thank you so much for that, just trying to keep the waterfall 🟢 where we can :)

@nbbeeken nbbeeken requested a review from dariakp June 8, 2021 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants